Skip to content

fix(files): escape user filenames in the Content-Disposition header - #6430

Merged
waleedlatif1 merged 1 commit into
stagingfrom
content-disposition-escaping
Aug 8, 2026
Merged

fix(files): escape user filenames in the Content-Disposition header#6430
waleedlatif1 merged 1 commit into
stagingfrom
content-disposition-escaping

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • escape the user-supplied filename before it is interpolated into the quoted filename parameter, so it cannot terminate the parameter or append parameters of its own
  • always percent-encode the filename* ext-value fully, including the characters encodeURIComponent leaves raw
  • filenames that are already safe printable ASCII keep their exact previous header, so ordinary downloads are unchanged
  • a control character in a name no longer produces an invalid header value (that previously made the download 500)

Type of Change

  • Bug fix

Testing

Added unit tests covering each case, including that an ordinary filename is byte-identical to before. app/api/files suite passes (247 tests); type-check and lint clean.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@waleedlatif1
fix(files): escape user filenames in the Content-Disposition header
bbeee90 
The download name is the user's originalName, which only rejects path
separators, so a quote reached the quoted filename parameter unescaped and could
close it and append parameters of its own. An injected filename* is the one that
matters: RFC 6266 tells clients to prefer it, so it decides the name the file
lands under on disk regardless of what the UI showed.

- neutralize the quote, backslash and non-printable characters in the quoted
  parameter, and neutralize the semicolon there too since that fallback exists
  for clients liable to split parameters without honouring the quoting
- percent-encode the filename* ext-value fully, including the characters
  encodeURIComponent leaves raw — the apostrophe is the ext-value delimiter
- names that are already safe printable ASCII keep their exact previous header

Also stops a control character in a name from producing an invalid header value,
which previously made the download 500.
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 8, 2026 7:30pm

Request Review

@cursor

cursor Bot commented Aug 8, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches security-sensitive download response headers driven by user-supplied names; the change is localized to filename encoding with thorough tests, limiting blast radius.

Overview
Hardens encodeFilenameForHeader so attacker-controlled download names cannot break or inject Content-Disposition parameters when files are served.

The quoted filename value is now built from sanitized ASCII (stripping ", \\, ;, and non-printable characters), and unsafe names also emit a single filename* ext-value via encodeExtValue, which percent-encodes characters encodeURIComponent leaves raw (notably ' in the UTF-8'' delimiter). Safe printable ASCII names still get the same filename="..." header as before.

Control characters in a name no longer produce invalid header values that could fail the response. Unit tests cover injection via quotes/filename*, apostrophes, CRLF, and ordinary filenames.

Reviewed by Cursor Bugbot for commit bbeee90. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR hardens file-download Content-Disposition construction against parameter injection and invalid header characters while preserving existing output for safe ASCII filenames.

  • Sanitizes the quoted filename fallback.
  • Fully percent-encodes RFC 8187 filename* values.
  • Adds regression coverage for injection payloads, control characters, Unicode, apostrophes, unchanged safe names, and storage-key prefixes.

Confidence Score: 5/5

The PR appears safe to merge with no actionable defects identified.

The revised encoding safely neutralizes quoted-parameter injection, emits valid percent-encoded extended values, and preserves the previous header for ordinary safe filenames.

Important Files Changed
Filename Overview
apps/sim/app/api/files/utils.ts Safely constructs quoted and extended filename parameters without exposing a concrete regression.
apps/sim/app/api/files/utils.test.ts Adds focused regression tests covering unsafe, Unicode, control-character, and ordinary filename behavior.

Reviews (1): Last reviewed commit: "fix(files): escape user filenames in the..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 merged commit a2ad4b6 into staging Aug 8, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the content-disposition-escaping branch August 8, 2026 19:33
@waleedlatif1 waleedlatif1 mentioned this pull request Aug 8, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1